Skip to content

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Sep 11, 2025

Eliminate doesOpcodeNeedPredicate and instead have emitPredicateMatch return true if any predicates were generated. Delegate actual predicate generation in emitPredicateMatch to SubtargetFeatureInfo::emitMCPredicateCheck. Additionally, remove the redundant parenthesis around the predicate conditions in the generated checkDecoderPredicate function.

Note that for ARM/AMDGPU this reduces the total # of predicates generated by a few. It seems the old code would sometimes generate duplicate predicates which were identical in semantics but one had an extra pair of parentheses (i..e, X and (X)). emitMCPredicateCheck does not seems to have that issue.

@jurahul jurahul marked this pull request as ready for review September 12, 2025 12:12
@llvmbot
Copy link
Member

llvmbot commented Sep 12, 2025

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

Use ListSeparator to join individual predicated with &&. Eliminate doesOpcodeNeedPredicate and instead have emitPredicateMatch return true if any predicates were generated.


Full diff: https://github.com/llvm/llvm-project/pull/158140.diff

1 Files Affected:

  • (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+7-25)
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 8747d02ac892b..96957aed60df3 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -635,8 +635,6 @@ class DecoderTableBuilder {
 
   bool emitPredicateMatch(raw_ostream &OS, unsigned EncodingID) const;
 
-  bool doesOpcodeNeedPredicate(unsigned EncodingID) const;
-
   void emitPredicateTableEntry(unsigned EncodingID) const;
 
   void emitSoftFailTableEntry(unsigned EncodingID) const;
@@ -1221,7 +1219,8 @@ bool DecoderTableBuilder::emitPredicateMatch(raw_ostream &OS,
                                              unsigned EncodingID) const {
   const ListInit *Predicates =
       Encodings[EncodingID].getRecord()->getValueAsListInit("Predicates");
-  bool IsFirstEmission = true;
+  ListSeparator LS(" && ");
+  bool AnyPredicate = false;
   for (unsigned i = 0; i < Predicates->size(); ++i) {
     const Record *Pred = Predicates->getElementAsRecord(i);
     if (!Pred->getValue("AssemblerMatcherPredicate"))
@@ -1230,28 +1229,13 @@ bool DecoderTableBuilder::emitPredicateMatch(raw_ostream &OS,
     if (!isa<DagInit>(Pred->getValue("AssemblerCondDag")->getValue()))
       continue;
 
-    if (!IsFirstEmission)
-      OS << " && ";
+    AnyPredicate = true;
+    OS << LS;
     if (emitPredicateMatchAux(*Pred->getValueAsDag("AssemblerCondDag"),
                               Predicates->size() > 1, OS))
       PrintFatalError(Pred->getLoc(), "Invalid AssemblerCondDag!");
-    IsFirstEmission = false;
-  }
-  return !Predicates->empty();
-}
-
-bool DecoderTableBuilder::doesOpcodeNeedPredicate(unsigned EncodingID) const {
-  const ListInit *Predicates =
-      Encodings[EncodingID].getRecord()->getValueAsListInit("Predicates");
-  for (unsigned i = 0; i < Predicates->size(); ++i) {
-    const Record *Pred = Predicates->getElementAsRecord(i);
-    if (!Pred->getValue("AssemblerMatcherPredicate"))
-      continue;
-
-    if (isa<DagInit>(Pred->getValue("AssemblerCondDag")->getValue()))
-      return true;
   }
-  return false;
+  return AnyPredicate;
 }
 
 unsigned DecoderTableBuilder::getPredicateIndex(StringRef Predicate) const {
@@ -1269,15 +1253,13 @@ unsigned DecoderTableBuilder::getPredicateIndex(StringRef Predicate) const {
 }
 
 void DecoderTableBuilder::emitPredicateTableEntry(unsigned EncodingID) const {
-  if (!doesOpcodeNeedPredicate(EncodingID))
-    return;
-
   // Build up the predicate string.
   SmallString<256> Predicate;
   // FIXME: emitPredicateMatch() functions can take a buffer directly rather
   // than a stream.
   raw_svector_ostream PS(Predicate);
-  emitPredicateMatch(PS, EncodingID);
+  if (!emitPredicateMatch(PS, EncodingID))
+    return;
 
   // Figure out the index into the predicate table for the predicate just
   // computed.

@jurahul jurahul force-pushed the decoder_predicate_cleanup branch from a6e9032 to c1f1dec Compare September 15, 2025 00:15
Copy link

github-actions bot commented Sep 15, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Use `ListSeparator` to join individual predicated with `&&`.
Eliminate `doesOpcodeNeedPredicate` and instead have
`emitPredicateMatch` return true if any predicates were generated.
@jurahul jurahul force-pushed the decoder_predicate_cleanup branch from c1f1dec to 328f1f3 Compare September 16, 2025 01:20
Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@s-barannikov
Copy link
Contributor

(The description needs an update.)

@jurahul
Copy link
Contributor Author

jurahul commented Sep 16, 2025

(The description needs an update.)

Updated, thanks

@jurahul jurahul merged commit b5e06b5 into llvm:main Sep 16, 2025
9 checks passed
@jurahul jurahul deleted the decoder_predicate_cleanup branch September 16, 2025 02:23
}
}
return true;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, missed it. emitPredicateMatchAux is now unused.

@jurahul
Copy link
Contributor Author

jurahul commented Sep 16, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants